Skip to content

Fix useHandleCallback compatibility with React.StrictMode#10486

Merged
djhi merged 2 commits intomasterfrom
fix/useHandleCallback_compatible_StrictMode
Jan 31, 2025
Merged

Fix useHandleCallback compatibility with React.StrictMode#10486
djhi merged 2 commits intomasterfrom
fix/useHandleCallback_compatible_StrictMode

Conversation

@erwanMarmelab
Copy link
Contributor

@erwanMarmelab erwanMarmelab commented Jan 30, 2025

Problem

Many authentication providers do not accept calling their methods for validating authentication callbacks multiple times. We had the case with ra-auth-auth0 and ra-keycloak (maybe even ra-supabase).

Our solution until now was to implement the necessary logic in the authProvider implementation (storing ongoing promise in the authProvider closure). This is fine but it means custom implementations must do it themselves too and this is not trivial for many.

Solution

Make the useHandleCallback resilient to StrictMode. Implement the logic that won’t call authProvider.handleCallback if another call is already ongoing.

How To Test

Revert the fix in useHandleAuthCallback.ts and run the test "should only call handleCallback once when the component is rendered" in useHandleAuthCallback.spec.tsx.

Additional Checks

  • The PR targets master for a bugfix, or next for a feature
  • The PR includes unit tests (if not possible, describe why)
  • [ ] The PR includes one or several stories (if not possible, describe why)
  • [ ] The documentation is up to date

Screenshot

Test without the fix:

image

@erwanMarmelab erwanMarmelab added the RFR Ready For Review label Jan 30, 2025
Comment on lines +34 to +55
handleCallbackPromise = new Promise(async (resolve, reject) => {
if (authProvider) {
if (typeof authProvider.handleCallback === 'function') {
try {
const result =
await authProvider.handleCallback({
signal,
});
return resolve(result ?? null);
} catch (error) {
return reject({
redirectTo: false,
message: error.message,
});
}
}
return resolve();
}
return reject({
message: 'Failed to handle login callback.',
});
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes the previous behavior. Please revert to the original code. Only keep the handleCallbackPromise creation as you did

@djhi djhi added this to the 5.5.3 milestone Jan 31, 2025
@djhi djhi merged commit 6ef98e7 into master Jan 31, 2025
16 checks passed
@djhi djhi deleted the fix/useHandleCallback_compatible_StrictMode branch January 31, 2025 10:12
@djhi djhi changed the title Make useHandleCallback compatible with StrictMode Fix useHandleCallback compatibility with React.StrictMode Jan 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

RFR Ready For Review

Development

Successfully merging this pull request may close these issues.

2 participants